NetNews Offline 2/NetNews Offline Volume 2.iso/news/comp/lang/c-part1/1897

Internet Message Format | 1996-08-05 | 4.9 KB

Path: beach.and.nl!usenet
From: [e-mail] (Jos A. Horsmeier)
Newsgroups: comp.lang.c
Subject: Re: Newbie question:  Is this code OK?
Date: 17 Jan 1996 16:18:33 GMT
Organization: AND Operations Research B.V.
Message-ID: <4dj7gp$[e-mail]>
References: <4di986$[e-mail]>
NNTP-Posting-Host: klepzeiker.and.nl
Mime-Version: 1.0
Content-Type: Text/Plain; charset=ISO-8859-1
X-Newsreader: WinVN 0.99.5

In article <4di986$[e-mail]>, [e-mail] wrote:

[ word counting program: ]

|I know it's usually not a good idea to increment a loop counter within a
|loop, but I have 2 nested loops using the same counter.  Was this a bad
|thing to do?
|
|I have an if statement and a for loop that test for the same things.  Is
|it possible to somehow combine these into one statement?  I couldn't
|think of a way to do it.
|
|Do I need the empty {} after a for loop that has no body?  I assumed if I
|didn't, the loop would execute the next statement after it.
|
|int words (char string[80], int length)
|{
|        int i, num_words = 0;
|
|        for (i=0; i<length; i++)
|        {                               /* if char isn't whitespace */
|                if ((string[i]!='\n') &&
|                        (string[i]!='\t') &&
|                        (string[i]!=' '))
|                {
|                        num_words++;            /* Increment num_words */
|                        for (; i<length, ((string[i]!='\n') &&
|                                (string[i]!='\t') &&
|                                (string[i]!=' ')); i++)
|                                 /* and skip past any other */
|                                 /* chars that may be there */
|                        { }     /* loop does all work, nothing in body */
|                }
|        }
|        return (num_words);
|}

First let me answer your question about the empty statement in the
second for loop: yes indeed, you do need some sort of a statement
there (the syntax requires it). The simplest form of an empty statement
is:

    for ( <init_part> ; <condition> ; <increment> )
        ; /* a single semi colon */

but your '{}' solution works just as fine. About your code -- as far
as I can see everything works fine indeed. Some remarks apply though:

You're testing for white space characters. Have a look at the isspace()
function; this function returns a non-zero value if its argument is
one of: space, tab, carriage return, new-line, form feed and vertical tab.

Have a look at the logic behind your program:

    as long as we're reading spaces, nothing happens
    else increment a counter and read past all the non-space
    characters until we hit another space character or the end of line.

Your program is always in on of the two possible states:

    it reads space characters or    (state A)
    it reads non-space characters    (state B)

If the program is running in state A and the next character to be read
happens to be a non-space character, the program moves to state B. If
it runs in state B and reads a space character, the program moves back
again to state A. Otherwise, no state transition occurs.

If the program moves from state A to state B, it just has read the start
of a new word. Given these observations, we can write another version
of your program in pseudo code like this:

    state=        stateA;
    num_words= 0;

    while more characters to read {

        c= current character;

        if (isspace(c)) {
            if (state == stateB)
                state= stateA;
        }
        else
            if (state == stateA) {
                num_words++;
                state= stateB;
            }
    }

Have a look at the if-then-else part -- we can 'optimize' the state
transitions a bit:

        if (isspace(c)) {
            state= stateA;
        else if (state == stateA) {
            num_words++;
            state= stateB;
        }

Let's turn this pseudo cody thingy into a C function; we represent stateA
by an int 1 (or any other non-zero value) and stateB by an int 0. While
we're at it, in C a string is usually represented by a zero terminated
array of characters; lets stick to this parlance:

    int    words(char* string) {

    int    state= 0;
    int    num_words= 0;

        for (; *string; string++)

            if (isspace(*string))
                state= 1;
            else if (state) {
                num_words++;
                state= 0;
            }

        return num_words;
    }

As you can see this revised function does the same amount of work as
your version, i.e. it reads every character once until everything is
read, but the 'functionality' of it all is not scattered around different
for loops and if statements.

If we really carry things over the edge, we could've implemented the
if-then-else part as:

    if (isspace(*string)^state) {
        num_words+= state;
        state= !state;
    }

but I wouldn't encourage such type of programming (leave that to idiots
who don't know any better, like me ;-)

I don't want to criticize your code at all, after all you're just starting
and you're doing a fine job, but you've asked for comments, so here you
got them ;-)

kind regards,

Jos aka [e-mail]
--
Atnwgqkrl gy zit vgksr, ug qshiqwtzoeqs!